❗ Release notes required

✅ Found changes and release notes in following paths:

Change path	Release notes path	Description
src/Compiler	docs/release-notes/.FSharp.Compiler.Service/11.0.0.md

Yet another bug. This needs a careful look.

I need to run some benchmarks again, too and compare standalone builds with the original Vlad's implementation.

@majocha:
If we separated the solved and unsolved form of a typar, why wouldn't we want to cache (type subsumption especially) for both?
.e.g. constraints might be already there and the cache would hold info on type subsumption of generic typars.

It would equally help if two typars are both unsolved, but strip down to the same form.

Wasn't the mistake more the fact that solved/unsolved form of the same TType_var had always the same key (based on Stamp) ?

This is definitely a safe fix, but I wonder if it doesn't too much limit the potential of the cache for typars.

Wasn't the mistake more the fact that solved/unsolved form of the same TType_var had always the same key (based on Stamp) ?

Yes, that makes sense. It makes me wonder if we could also handle unsolved nullness better.

Nullness is not stamped, so in the case of unresolved/not yet resolved/ nullness, there is no identifier to hold onto.
The identity of the NullnessVar (possibly the leaf one if multiple are sequentially linked) object itself (via https://learn.microsoft.com/en-us/dotnet/api/system.runtime.compilerservices.runtimehelpers.gethashcode?view=net-9.0 )

This fix is accidental, the real problem is here:

Wasn't the mistake more the fact that solved/unsolved form of the same TType_var had always the same key (based on Stamp) ?

Yes, in fact in case of typars we don't want the stamp as identity at all. We should use the structure of the solution (TType) if available or a common token for unsolved.

I'm testing this in the IDE with the notorious OpenTK 5.0. It improved things significantly, there are way less cache entries now but much more hits, resulting in 99% ratio with little memory use and no constant churn during edits:

|             Cache name              | hit-ratio | adds | updates |  hits   | misses | 
|-------------------------------------|-----------|------|---------|---------|--------|
| typeSubsumptionCache                |    99.94% | 2752 |       0 | 4618992 |   2752 |

The huge mistake that causes a lot of inefficiency was emitting typar stamps as part of the cache key. This causes a lot of equivalent but really unusable keys polluting the cache. The increased churn is especially heavy on the IDE. I cannot see a solution to this, that would guarantee soundness.

The huge mistake that causes a lot of inefficiency was emitting typar stamps as part of the cache key. This causes a lot of equivalent but really unusable keys polluting the cache. The increased churn is especially heavy on the IDE. I cannot see a solution to this, that would guarantee soundness.

What about skipping cache for them?
For unsolved typars, the typefeasiblysubsubsumestypes is a quick decision in case of TType_var anyway.

Of course it somewhat limits the genericity of the cache, and is special-tailored to the place of application.

The typesubsumption was really needed for concrete types, with super types and interface hierarchies.
Maybe if Ttype_var is not part of it (after stripping), we can have both correctness and good cache contents?

I was thinking of content-based hashing, but you would need to hash all type constraints content-wise as they are mutable as well. Maybe the inherent mutation for solutions and constraints of a type var are good enough reasons to treat them differently?
(mutation is not well suited for cache keys)

So, basically, we care only about caching TType_app _ pairs.
But most of them have type vars. Often still unsolved when they end up in the checkSubsumes call, just like in the test here.
I tried to not cache the types with unsolved vars but the results are abysmal, looking at OpenTK 5.0 project as baseline build times go from 10 sec to forever.

I wonder if the constraints are really a problem wrt to type subsumption cache. Because the current situation where we emit only a stamp as typar identity worked so far ( the bug this PR tries to fix is elsewhere). A given typar can very well have its constraints mutated while the stamp stays constant, so it is already not collision free in general. It just does not collide in practice for this usage. I think it was like this since the original Vlad's implementation.

Copilot when asked says:

Q: which typar constraints can influence the result of TypeFeasiblySubsumesType? I wonder because I need to know what should take part in keying the TypeSubsumptionCache.

A: Short answer: none of the typar constraints affect TypeFeasiblySubsumesType.
Details:
• In TypeFeasiblySubsumesType, if either side is a type variable (TType_var), the function immediately returns true and does not consult the variable’s constraints.
• For nominal types (TType_app) with the same tycon, it delegates to TypesFeasiblyEquiv for each type argument. TypesFeasiblyEquiv also returns true when it encounters a TType_var (and ignores its constraints), and treats measure types as feasibly equivalent without comparing the actual measures.
• For tuples/anon/funs, it uses TypesFeasiblyEquiv similarly, so constraints are again ignored.
• The remaining branches follow nominal inheritance and interface chains of the concrete right-hand type. This depends on the type definitions/instantiations (and ultimately on metadata reachable via g/amap), not on typar constraint sets.
Implications for the cache key:
• You do not need to include any typar constraint environment in the TypeSubsumptionCache key.
• The existing key of stripped nominal type structures (when both sides are TType_app) plus CanCoerce is correct. This aligns with how the algorithm makes decisions.
• The cache is already partitioned per TcGlobals via the weak map, so differences in loaded metadata are naturally isolated.

hmm.

I quickly asked copilot:

Q: Does TypeFeasiblySubsumesType take nullness into account ?

A:

Short answer: No. The predicate is intentionally nullness-agnostic.
Why:
• In the core match it compares constructed types as TType_app (tc1, l1, _), TType_app (tc2, l2, ) and discards the third field (), which is where the nullness marker lives. Only tycon equality and argument feasibility are checked via TypesFeasiblyEquiv, which likewise ignores nullness.
• The special-case for object uses isObjTyAnyNullness g ty1, i.e., any nullness of obj is accepted as a supertype of any reference type, again ignoring nullness compatibility.
• Supertype/interface traversal (GetSuperTypeOfType, GetImmediateInterfacesOfType) calls back into TypeFeasiblySubsumesType without reconciling nullness.
Related:
• FindUniqueFeasibleSupertype preserves the nullness of ty2 in the result (addNullnessToTy n2 t), but the feasibility test itself still uses TypeFeasiblySubsumesType and thus ignores nullness.
• ChooseTyparSolution may add nullness due to constraints, but that’s orthogonal to the subsumption check.
If you need a nullness-aware check, compare nullnessOfTy g ty1 and nullnessOfTy g ty2 explicitly alongside calling TypeFeasiblySubsumesType.